Aggregate #97
Merged
Aggregate #97
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
May 29, 2024 15:21
rwijtvliet
reviewed
Jul 3, 2024
portfolyo/tools/duration.py
Outdated
| @@ -14,7 +14,7 @@ def stamp(ts: pd.Timestamp, freq: str) -> tools_unit.Q_: | |||
| ---------- | |||
| ts : pd.Timestamp | |||
| Timestamp for which to calculate the duration. | |||
| freq : {{{', '.join(tools_freq.FREQUENCIES)}}} | |||
| freq : {{{tools_freq.ALLOWED_FREQUENCIES_DOCS}}} | |||
There was a problem hiding this comment.
I think we only need single (not tripple) braces
portfolyo/tools/freq.py
Outdated
| # assert_freq_valid(freq_source) | ||
| # assert_freq_valid(freq_target) | ||
| # Compare if the freq are the same | ||
| if freq_source == freq_target: |
There was a problem hiding this comment.
This check after conversion to offset? (In case e.g. freqs are "QS" and "QS-JAN")
There was a problem hiding this comment.
done, also added case-test in tests
portfolyo/tools/freq.py
Outdated
| ) | ||
|
|
||
|
|
||
| def up_or_down2(freq_source: str, freq_target: str) -> int: |
There was a problem hiding this comment.
Since we no longer have up_or_down, maybe we can rename this back to up_or_down? And move it down in the file, so assert_sufficiently_long is next to assert_sufficiently_short and assert_equally_long.
portfolyo/tools/freq.py
Outdated
| # we are in the case QS and QS | ||
| return 0 | ||
|
|
||
| raise ValueError |
There was a problem hiding this comment.
Can you add a descriptive message raised with with error, so the user knows what he did wrong?
rwijtvliet
approved these changes
Jul 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Aggregation to non-calendar periods #57
Added freq "QS-FEB","QS-MAR", etc, "AS-FEB", "AS-MAR" as allowed.
Defined aggregation rules between them
📚 Documentation preview 📚: https://portfolyo--97.org.readthedocs.build/en/97/